Skip to content

[ui-core]: Improve login redirect to capture exact browser URL as next param #4297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[ui-core]: Improve login redirect to capture exact browser URL as next param #4297

wants to merge 6 commits into from
+86 −59

Conversation

@ramprasadagarwal
Copy link
Collaborator

@ramprasadagarwal ramprasadagarwal commented Oct 27, 2025

What changes were proposed in this pull request?

  • When the session times out the next url was not encoded by default. Where as when the login is success the backend decodes the next param by default which leads to inconsistency.
  • This PR fixes this bug by encoding the browser URL before passing it to the login next param.

How was this patch tested?

  • Manually tested

Please review Hue Contributing Guide before opening a pull request.

@ramprasadagarwal ramprasadagarwal self-assigned this Oct 27, 2025
@github-actions
Copy link

github-actions bot commented Oct 27, 2025 •
edited
Loading

✅ Test files were modified. Ensure that the tests cover all relevant changes. ✅

@github-actions
Copy link

github-actions bot commented Oct 27, 2025 •
edited
Loading

UI Code Coverage Report

Lines Statements Branches Functions
Coverage: 33%
 39.89% (31312/78478) 31.57% (14614/46286) 25.9% (2357/9097)

@github-actions
Copy link

github-actions bot commented Oct 27, 2025 •
edited
Loading

Coverage

Backend Code Coverage Report •
FileStmtsMissCoverMissing
desktop/core/src/desktop
   middleware.py57724258%86–89, 91, 93–101, 103, 121, 126–134, 136, 151, 175, 178, 207–210, 233, 261, 286, 289–291, 294, 297–299, 302–307, 311–313, 315–317, 321–323, 326–331, 334–337, 339–340, 342–350, 353–356, 360–363, 368–369, 371–377, 389, 403, 406, 430–432, 460, 465–467, 469–470, 472–475, 484, 494, 516, 519, 537, 546, 559–562, 565–570, 573, 576–579, 591, 594–596, 599–600, 607–614, 616–617, 622–625, 632–638, 681, 695, 703–704, 713–716, 720–723, 725, 727, 729–731, 734–735, 744, 752–754, 756–757, 774–784, 787–793, 800–806, 822–823, 851, 874, 884, 893–894, 906–907, 910–912, 915–918, 921–924, 926–928, 930, 934–935, 937, 940, 942, 955–956, 960
TOTAL569172893949% 

bjornalm
bjornalm reviewed Oct 28, 2025
View reviewed changes
desktop/core/src/desktop/templates/redirect_to_login.mako
Comment on lines +24 to +25
var browserUrl = window.location.pathname + window.location.search + window.location.hash;
window.location.href = '${login_url}?${redirect_field_name}=' + encodeURIComponent(browserUrl);
Copy link
Collaborator

@bjornalm bjornalm Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranade1 What do you think, is it possible somehow to achieve this in middleware.py‎ so that we still can rely on standard browser redirects instead of JS redirect?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bjornalm bjornalm bjornalm left review comments

@tabraiz12 tabraiz12 Awaiting requested review from tabraiz12

@JohanAhlen JohanAhlen Awaiting requested review from JohanAhlen

@Harshg999 Harshg999 Awaiting requested review from Harshg999

At least 2 approving reviews are required to merge this pull request.

Assignees

@ramprasadagarwal ramprasadagarwal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ramprasadagarwal @bjornalm